-
Notifications
You must be signed in to change notification settings - Fork 469
Add flag for preserve jsx mode to rescript.json #7547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
pub fn get_jsx_preserve_args(&self) -> Vec<String> { | ||
match self.jsx.to_owned() { | ||
Some(jsx) => match jsx.preserve { | ||
Some(true) => vec!["-bs-jsx-preserve".to_string()], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not any new options with -bs
prefix (and get rid of the old ones eventually).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the BSC flag; it was already in place. Why didn't you speak up when it was introduced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. Must have overlooked it at that point.
@jfrolich just try the pkg-pr version of this PR and the setting is not getting picked up. |
I'm a bit confused why the other jsx compiler arguments are currently not passed down. |
@jfrolich I tried this after building locally,
cargo build --release
.Then
../rescript/rewatch/target/release/rewatch
in my own project, and this didn't get picked up. Any ideas why?